Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.84%. Comparing base (8bc7c1e) to head (ed0c23e).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff            @@
##               main    #1243   +/-   ##
=========================================
  Coverage     77.84%   77.84%           
  Complexity     1223     1223           
=========================================
  Files           103      103           
  Lines          5778     5778           
  Branches        599      599           
=========================================
  Hits           4498     4498           
  Misses          992      992           
  Partials        288      288           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

So, we'll never merge this PR as it requires the security integ test. That's broken until #1251 gets merged.

But #1251 is dependent on a fix in ML Commons and they've already bumped to 3.4.0

So easy fix is to bump version in #1251, which will make this PR redundant. .... unless @peterzhuamazon or @gaiksaya thinks there's a way to override a required test :)

Edge cases FTW!

#1251 should not be a blocker here as there are no resource-sharing framework related changes present in this repo just yet.

this PR is blocked by ml-commons PR: opensearch-project/ml-commons#4402 (and here as well: opensearch-project/ml-commons#4333)

Once either of these are merged this PR should be unblocked and then #1251

#1251 should not be a blocker here as there are no resource-sharing framework related changes present in this repo just yet.

this PR is blocked by ml-commons PR: opensearch-project/ml-commons#4402 (and here as well: opensearch-project/ml-commons#4333)

Once either of these are merged this PR should be unblocked and then #1251

@DarshitChanpura @cwperks This bump is still failing on ML commons related failures following the merge of the above:

»  fatal error in thread [main], exiting
»  java.lang.InstantiationError: org.opensearch.security.spi.resources.ResourceProvider
»  	at org.opensearch.ml.resources.MLResourceSharingExtension.getResourceProviders(MLResourceSharingExtension.java:22)
»  	at org.opensearch.security.resources.ResourcePluginInfo.setResourceSharingExtensions(ResourcePluginInfo.java:76)
»  

What else can we do on this repo to get this version bump in?

So easy fix is to bump version in #1251, which will make this PR redundant. .... unless @peterzhuamazon or @gaiksaya thinks there's a way to override a required test :)

I can hard-merge. Let me know if you want to do that

So easy fix is to bump version in #1251, which will make this PR redundant. .... unless @peterzhuamazon or @gaiksaya thinks there's a way to override a required test :)

I can hard-merge. Let me know if you want to do that

Let's wait a few days. The line number is 2-day old code in ML Commons so the latest change apparently hasn't made its way to the CI Image used for the security test....

Let's wait a few days. The line number is 2-day old code in ML Commons so the latest change apparently hasn't made its way to the CI Image used for the security test....

https://hub.docker.com/r/opensearchstaging/ci-runner last updated 4 days ago...

Let's wait a few days. The line number is 2-day old code in ML Commons so the latest change apparently hasn't made its way to the CI Image used for the security test....

hub.docker.com/r/opensearchstaging/ci-runner last updated 4 days ago...

Those images do not change frequently. @peterzhuamazon do we need to make any infra change here?

Let's wait a few days. The line number is 2-day old code in ML Commons so the latest change apparently hasn't made its way to the CI Image used for the security test....

hub.docker.com/r/opensearchstaging/ci-runner last updated 4 days ago...

Those images do not change frequently. @peterzhuamazon do we need to make any infra change here?

Hi @dbwiddis what exact change are we talking about here?
Our images has already updated to JDK25 as default.
The only thing hasnt change on 3.4.0 is manifest switching to jdk25 but that shouldnt have any issues to compile on jdk24 anyway. Thanks.

Hi @dbwiddis what exact change are we talking about here? Our images has already updated to JDK25 as default.

@peterzhuamazon the (required) security test workflow is failing. It uses this container image at hub.docker.com/r/opensearchstaging/ci-runner.

Here's where it's called:

It's used across many plugins.

The current test is failing based on an ML Commons bug fixed 2 days ago but the last docker image was published 5 days ago. (The same test at the neural search repository is also failing for the same reason. CC: @martin-gaievski).

Would love that image to be updated soon to unblock this version bump and pretty much any PR on this repo.

Would suggest you consider more frequent updates of an image used in CI.

@dbwiddis the image has nothing to do with ML plugin fix nor does it store any snapshots.
It is just an image with necessary runtime and packages for supporting the run and is also used on Jenkins during build process.

Is there a specific package you are expecting?

Seems like the publishing to maven snapshots failed: https://github.com/opensearch-project/ml-commons/actions/runs/19123769455

Pending this: opensearch-project/ml-commons#4405

Thanks for clearing it up.

I was confused because I stopped seeing the same error on the other tests so assumed something had been updated somewhere

Thanks for clearing it up.

I was confused because I stopped seeing the same error on the other tests so assumed something had been updated somewhere

No problem, merged now and published: https://github.com/opensearch-project/ml-commons/actions/runs/19237274367